Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Incorporate video link with headings #985

Merged
merged 9 commits into from
Jul 10, 2024
Merged

Conversation

jasongao97
Copy link
Contributor

Implementation of #946

Without Image

image

=>

image

With Image

image

=>

image

Copy link

netlify bot commented Jul 2, 2024

Deploy Preview for nature-of-code-2nd-edition ready!

Name Link
🔨 Latest commit d9d87aa
🔍 Latest deploy log https://app.netlify.com/sites/nature-of-code-2nd-edition/deploys/668d604e1e8e2100083ded3e
😎 Deploy Preview https://deploy-preview-985--nature-of-code-2nd-edition.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jul 2, 2024

Deploy Preview for nature-of-code-pdf ready!

Name Link
🔨 Latest commit d9d87aa
🔍 Latest deploy log https://app.netlify.com/sites/nature-of-code-pdf/deploys/668d604e68e0660008099670
😎 Deploy Preview https://deploy-preview-985--nature-of-code-pdf.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@shiffman
Copy link
Member

shiffman commented Jul 3, 2024

This is great, thank you @jasongao97! Two comments / questions:

  • I believe we had discussed not putting the video thumbnail images in Notion but rather just including them in a directory on github. That said, I don't remember why I thought we should do it this way (maybe to reduce the amount of "clutter" in Notion? Or knowing that eventually we could automate retrieving them from the API?" What do you think?
  • It all looks great but I wonder if a overlaying a "play" button icon in the middle of the thumbnail hover pop-up would help make it more clear it will play a video?
  • Can we set the target for the link to open in a "new tab" automatically for the videos?

@jasongao97
Copy link
Contributor Author

I believe we had discussed not putting the video thumbnail images in Notion but rather just including them in a directory on github. That said, I don't remember why I thought we should do it this way (maybe to reduce the amount of "clutter" in Notion? Or knowing that eventually we could automate retrieving them from the API?" What do you think?

Thank you for bringing this up! I agree that fetching the screenshots from the API would simplify the process and reduce clutter in Notion. Currently, I'm retrieving them from Notion since the URLs are pointing to the Coding Train's website instead of YouTube. I'm curious, though, if there might be future cases where the video links point to somewhere else. Do you think that's a possibility?

It all looks great but I wonder if a overlaying a "play" button icon in the middle of the thumbnail hover pop-up would help make it more clear it will play a video?

How about this?

image

Can we set the target for the link to open in a "new tab" automatically for the videos?

Certainly! Added.

@shiffman
Copy link
Member

shiffman commented Jul 5, 2024

Thank you for bringing this up! I agree that fetching the screenshots from the API would simplify the process and reduce clutter in Notion. Currently, I'm retrieving them from Notion since the URLs are pointing to the Coding Train's website instead of YouTube. I'm curious, though, if there might be future cases where the video links point to somewhere else. Do you think that's a possibility?

I think (for now) we can safely assume that the links will only ever point to thecodingtrain.com or youtube.com. Even though it's clunky, I think we should take the images out of Notion and just make a "hard coded" directory of video thumbnails here in the github repo. Would that work? Also, does the PDF build know to ignore the video elements in Notion?

How about this?

Yes, looks great! @tuantinghuang feel free to weigh in, we can always adjust the design later.

@jasongao97 if we can just remove the thumb from notion and bring it in here to this branch then I'll merge!

@shiffman
Copy link
Member

shiffman commented Jul 5, 2024

(Also, I'm going to take out the video number text in Notion, I think it's not important or relevant and just confuses things since the book has its own numbering system. The videos are just supplemental and appear wherever I happen to have something that matches what is being discussed in the text.)

@jasongao97
Copy link
Contributor Author

I think (for now) we can safely assume that the links will only ever point to thecodingtrain.com or youtube.com. Even though it's clunky, I think we should take the images out of Notion and just make a "hard coded" directory of video thumbnails here in the github repo. Would that work?

Yes, Got it!

Also, does the PDF build know to ignore the video elements in Notion?

I am adding this.

@jasongao97
Copy link
Contributor Author

@shiffman I am wondering if thumbnail file be named as the callout's title in snake-case with symbol removed?

e.g. the 'What is a vector?' in Chapter 1 video's file would be static/thumbnails/01_vectors/what-is-a-vector.png

@shiffman
Copy link
Member

shiffman commented Jul 5, 2024

Yes that sounds good!

@shiffman
Copy link
Member

shiffman commented Jul 6, 2024

Hi @jasongao97 I have a new idea! Take a look at @fturmel's automated download of YT thumbnails in CodingTrain/thecodingtrain.com#1646 as well as CodingTrain/thecodingtrain.com#1645 (comment). I wonder if we have the ID for each video embedded in Notion we could automate the retrieval of the thumb?

If it would simplify things (and maybe it's actually better for the end user experience to get to the YT page directly) we could embed the YT videos in Notion (then we have direct access to the video ID) rather than the Coding Train webpage?

What do you think?

@fturmel and @dipamsen, welcome your thoughts here as well!

@fturmel
Copy link
Contributor

fturmel commented Jul 6, 2024

If I understand correctly, this is only for hover states on the web version, no print/PDF? Will the videos linked always be known to the Coding Train site (either via YouTube or Coding Train links)?

Here's an idea: if you're OK sharing bandwidth between both sites, it would make sense to put the responsibility of the Coding Train site to recommend a thumbnail to use. We could generate a lookup/manifest file that has all urls, youtube ids and thumbnail images known to the site. The NoC site loads the manifest at runtime so it's always in sync. Images could also be loaded directly from thecodingtrain.com if we add the proper CORS header.

Something like this with O(1) lookups to the thumbnail index:

{
  "thumbnails": ["https://thecodingtrain.com/static/c8098520739212b37e99d0c131d4a6ba/bb8ee/index.png"],
  "urls": {
    "https://thecodingtrain.com/challenges/180-falling-sand": 0
  },
  "youtubeIds": {
    "L4u7Zy_b868": 0
  }
}

or just a simple array, slower lookups but the relationship between the data is kept:

[
  {
    "url": "https://thecodingtrain.com/challenges/180-falling-sand",
    "thumbnail": "https://thecodingtrain.com/static/c8098520739212b37e99d0c131d4a6ba/bb8ee/index.png",
    "youtubeId": "L4u7Zy_b868"
  }
]

@dipamsen
Copy link

dipamsen commented Jul 6, 2024

Based on my understanding, can confirm:

this is only for hover states on the web version, no print/PDF?

Yes

Will the videos linked always be known to the Coding Train site (either via YouTube or Coding Train links)?

Ideally, yes, the link will be either to a video page on the website, or a YouTube video, and the video should have a corresponding page on the site

@shiffman
Copy link
Member

shiffman commented Jul 7, 2024

If I understand correctly, this is only for hover states on the web version, no print/PDF? Will the videos linked always be known to the Coding Train site (either via YouTube or Coding Train links)?

This is correct. I am trying to decide whether the link should go to thecodingtrain.com or youtube.com, I'm still on the fence about this.

Here's an idea: if you're OK sharing bandwidth between both sites, it would make sense to put the responsibility of the Coding Train site to recommend a thumbnail to use. We could generate a lookup/manifest file that has all urls, youtube ids and thumbnail images known to the site. The NoC site loads the manifest at runtime so it's always in sync. Images could also be loaded directly from thecodingtrain.com if we add the proper CORS header.

Yes, I am fine with thecodingtrain.com being responsible for hosting / providing the thumb image for the hover. The Nature of Code site, however, is built in a non-traditional way via a GitHub action. The action pulls everything from a Notion database and builds the HTML pages. The goal here is to include a single URL in Notion (coding train page or youtube video).

@fturmel
Copy link
Contributor

fturmel commented Jul 7, 2024

What I was suggesting would work regardless of the NoC build system since it's all in-browser (client-side).

The lookup would be created during the Coding Train builds. The NoC site would load the lookup JSON once in browser and the VideoLink React component would then be able to determine the thumbnail to use based on the video URL and lookup. It would allow using Coding Train URLs in Notion without the need for additional thumbnail information. It also means the NoC codebase would not need to keep these mappings or images locally.

Also relevant, as long as you use a resolution of sddefault or lower (maxresdefault presence is not reliable), the image from the YT CDN can be used directly in the img src. If you only have YouTube links in Notion you already have enough information and no need for the lookup described in the previous paragraph.

WebP: https://img.youtube.com/vi_webp/${videoId}/${resolutionName}.webp
JPEG: https://img.youtube.com/vi/${videoId}/${resolutionName}.jpg

resolution name pixel resolution
maxresdefault 1280x720
sddefault 640x480
hqdefault 480x360
mqdefault 320x180
default 120x90

Hope this makes sense!


EDIT: Sorry for all the rambling, I'm going to stop now 🤪

I just wanted to clarify that while the fetch-notion repo does builds HTML, the NoC codebase also has its build phase and can augment these HTML elements with React components depending on their data-type attributes. At least that's how I understand the system from my somewhat limited exposure to it. That means that it's also possible to use a lookup file like what I described at NoC build time during the AST render phase and inject a thumbnail URL here.

const renderAst = ({ ast, images }) => {
visit(ast, { tagName: 'img' }, (node) => {
const relativePath = node.properties.src;
// If the image src exist as a local file
// use Gatsby Image to handle
const imageSharp = images.find(
(image) => image.relativePath === relativePath,
);
if (imageSharp) {
node.tagName = 'gatsby-image';
node.properties.image = imageSharp;
}
});
const processor = unified().use(rehypeReact, {
createElement: React.createElement,
Fragment: React.Fragment,
components: {
'gatsby-image': Image,
'embed-example': Example,
'video-link': VideoLink,
},
});
return processor.stringify(ast);
};

@fturmel
Copy link
Contributor

fturmel commented Jul 7, 2024

One last idea (for real this time), which I think is possibly the best one because it's simple and could be really useful outside of the scope of this integration.

We could add redirect shortcuts on the Coding Train site just like the numerical shortcuts for challenges (see CodingTrain/thecodingtrain.com#1556)

ex: https://thecodingtrain.com/yt/Rob0pbE7kkshttps://thecodingtrain.com/tracks/the-nature-of-code-2/noc/1-vectors/2-vector-math

The NoC project could store only the YouTube ID or URL in Notion. You then have enough information to link to either YouTube or the Coding Train shortcut (or both). And you can directly use the YouTube CDN image as I previously described.

@shiffman
Copy link
Member

shiffman commented Jul 8, 2024

One last idea (for real this time), which I think is possibly the best one because it's simple and could be really useful outside of the scope of this integration.

We could add redirect shortcuts on the Coding Train site just like the numerical shortcuts for challenges (see CodingTrain/thecodingtrain.com#1556)

ex: https://thecodingtrain.com/yt/Rob0pbE7kkshttps://thecodingtrain.com/tracks/the-nature-of-code-2/noc/1-vectors/2-vector-math

The NoC project could store only the YouTube ID or URL in Notion. You then have enough information to link to either YouTube or the Coding Train shortcut (or both). And you can directly use the YouTube CDN image as I previously described.

Yes, this is perfect! What do you think @jasongao97? I do think that we should pull the thumbnails and store them locally here at build time, however, rather than have the website embed the YouTube CDN link? We're doing that with everything that is in Notion already so might as well with YouTube images as well.

How do you envision automating the redirects @fturmel? For now @jasongao97 I think we should just embed the YT url and then we can update to use CT links later if we want. You can pull the ID from the url and then grab the thumbnail via the methods discussed in CodingTrain/thecodingtrain.com#1645.

@shiffman
Copy link
Member

shiffman commented Jul 8, 2024

I've updated the sample video for testing purposes in Notion FYI.

Screen Shot 2024-07-08 at 11 57 02 AM

@dipamsen
Copy link

dipamsen commented Jul 8, 2024

What will be finally linked on the NoC website? The YT video, or the page on theCodingTrain.com?

@shiffman
Copy link
Member

shiffman commented Jul 8, 2024

I'd like it to be possible to be either eventually but I think I'll start with YT video as in the sample.

@fturmel
Copy link
Contributor

fturmel commented Jul 8, 2024

How do you envision automating the redirects @fturmel?

@shiffman On the Coding Train site, my initial thought was just to create the YouTube ID to URL mapping at build time and dump them in the _redirects file like CodingTrain/thecodingtrain.com#1556. However, it is ~450 additional redirect rules that need to be processed for every single request. A Netlify engineer in 2020 gave a recommendation to keep them ideally under 1k, with a soft limit of 10k. I don't know how much latency that would end up introducing, but maybe it's the wrong path for us to take since we're already at about 230 (~50 hardcoded + ~180 numerical challenges shortcuts).

We were discussing Netlify Edge Functions yesterday, and it seems like it would be a good fit. The YouTube ID to URL mapping would still be created at build time but the output would be a JSON file that gets bundled and deployed with the function itself. We'd have one rewrite rule to add in _redirects, something like /yt/:youtubeId /.netlify/edge-functions/our-redirect-function 200!.

Because a YouTube video can appear in challenges and/or multiple tracks, we'll need to prioritize where the redirect goes. As was discussed back when we implemented the canonical track feature, we'd pick the first match in this order:

  1. Challenge
  2. Canonical track
  3. Track

For multi-part videos like the Rubik's Cube challenge, we can append the proper anchor to the destination (#part-2, etc).

The end result would be for example https://thecodingtrain.com/yt/Rob0pbE7kks redirecting to https://thecodingtrain.com/tracks/the-nature-of-code-2/noc/1-vectors/2-vector-math with a 302 status code. If the YouTube ID is not known to the site, we return a 404 instead.

The NoC site would simply link to https://thecodingtrain.com/yt/{youtubeId}.

@jasongao97
Copy link
Contributor Author

I just want to confirm my understanding. There are two types of URLs from Notion, and here's how they will be processed:

  1. URL pointing to a YouTube page:
    • link to YouTube.
    • Thumbnails are fetched from the YouTube CDN during the build process.
  2. URL pointing to a Coding Train page:
    • link to Coding Train.
    • Thumbnails are fetched from the Coding Train website during the build process.

Additionally: Since the download images action can be time-consuming and doesn't occur frequently, perhaps we could separate it into another GitHub action to be triggered as needed.

@fturmel
Copy link
Contributor

fturmel commented Jul 8, 2024

@jasongao97 That could work too. My mental model was more:

  • All video links on Notion are YouTube URLs
  • Thumbnails would be locally cached versions of https://img.youtube.com/vi_webp/${videoId}/hqdefault.webp or whatever format and resolution fits the context best
    • Note that a manual redeploy of the NoC site would be required if a thumbnail is updated on YouTube. Using the YouTube CDN directly would mitigate this and not slow down the builds.

And then if we go ahead and implement the https://thecodingtrain.com/yt/{videoId} redirects, the NoC site would have the option to link there instead or in addition to YouTube. The Notion sources wouldn't have to be modified (the behavior could happen in the VideoLink React component), and you can still use the YouTube thumbnails for both link types. For this to work, we're also assuming that all Notion video links would be to YouTube videos used by the Coding Train site in a track or challenge.

@shiffman
Copy link
Member

shiffman commented Jul 8, 2024

Yes what @fturmel suggests make sense to me. Let's make this work first with just YT video links directly. We can add support for ct.com links as a second step. if it simplifies things for the image to come from the YT CDN rather than a local copy I'm good with that! Id like to get this merged with the simplest working version as soon as possible so I can move onto running other site updates. Thank you all!!

@jasongao97
Copy link
Contributor Author

jasongao97 commented Jul 9, 2024

I've added the feature of displaying the thumbnail get from the YouTube CDN.

Preview Link: https://deploy-preview-985--nature-of-code-2nd-edition.netlify.app/vectors/#vector-addition

@shiffman
Copy link
Member

Fantastic!!!

@shiffman shiffman merged commit ce4235a into main Jul 10, 2024
9 checks passed
@shiffman shiffman mentioned this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants